Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

add option to eol just the package docs #1695

Closed
wants to merge 1 commit into from
Closed

add option to eol just the package docs #1695

wants to merge 1 commit into from
+2 −1

Conversation

samccann
Copy link
Contributor

Fixes #1490

@samccann samccann added backport-2.15 Automatically create a backport for the stable-2.15 branch backport-2.16 Automatically create a backport for the stable-2.16 branch backport-2.17 Automatically create a backport for the stable-2.17 branch labels Jul 15, 2024
webknjaz
webknjaz reviewed Jul 15, 2024
View reviewed changes
docs/docsite/rst/conf.py
# Change First false to true for core EOL, 2nd false to true for package EOL
'is_eol': False if tags.has('core') else False,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why can't you just add an additional tag and set it on the calling side? This would give you more control/flexibility with less hardcoding.

Suggested change
# Change First false to true for core EOL, 2nd false to true for package EOL
'is_eol': False if tags.has('core') else False,
'is_eol': tags.has('is_eol'),

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @webknjaz ! I'm trying to understand how this would work. So once a repo goes EOL, it's always published as eol. So instead of having one place here in the conf.py to set the eol flag, I'd have to set it in multiple places around https://github.com/ansible/ansible-documentation/blob/devel/docs/docsite/Makefile#L104 (the calling side) plus the nox commands?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If my understanding is correct, once the suggestion from @webknjaz is merged then you would need to update two places in the Makefile to EOL the package docs:

ansible-documentation/docs/docsite/Makefile

Line 104 in 3b43d64

htmldocs: ansible_structure generate_rst

and

ansible-documentation/docs/docsite/Makefile

Line 114 in 3b43d64

singlehtmldocs: ansible_structure generate_rst

For example, currently that the html target is this:

CPUS=$(CPUS) $(MAKE) -f Makefile.sphinx 'DOCS_VARIANTS=-t ansible' html

To EOL the package docs, you would update that line to something like this:

CPUS=$(CPUS) $(MAKE) -f Makefile.sphinx 'DOCS_VARIANTS=-t ansible -t is_eol' html

I don't think we would need to touch the nox commands at this stage. They only call the Makefile.

Ofc you'd need to then make other updates to the Makefile separately to EOL the core docs when the time comes. So maybe it's a little error-prone to do that. I do like the idea of having a central config where you can switch eol on or off.

Maybe it would be possible to add some logic to the version helper somehow: https://github.com/ansible/ansible-documentation/blob/devel/docs/docsite/version_helper.py

Having a list of EOL versions somewhere could work. From the maintainer perspective, adding a version to an EOL list at the appropriate time seems like a natural, intuitive approach. Just a thought.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@oraNod that config should probably live where make targets are being called, externally to the Makefile.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@samccann in general, the idea is that this is not to be hardcoded in the docs generator but parametrized. And those params would govern how this flag is to be computed.

@samccann
Copy link
Contributor Author

@oraNod @gotmax23 just looking for some opinions here or whether to do the PR as is or implement the suggestion above (which does require more changes per PR for each time we EOL a release).

@oraNod
Copy link
Contributor

oraNod commented Sep 22, 2024

@samccann I'd like to suggest another approach to this. What if we maintained a file in the repo that contains something like this?

active:
  - stable-2.17
  - stable-2.16
  - stable-2.15

eol:
  - stable-2.14

I would prefer a declarative approach to this for clarity and maintainability. It could also improve how we approach branching to stay in sync with core stable versions.

For the eol banner, I think we could use a sphinx role to inject the banner at build time for any version in the eol list. Not sure if that's overkill but it sounds like fun.

For core stable branches, we can periodically run a workflow that checks out the core repo, compares branches against the active list (basic idea), and pings the docs channel in matrix like we do for build failures. One of the maintainers can pick that up and do the branching. An announcement saves the need for someone to monitor the core repo and notice the new stable build.

I'm happy to take a stab at this but wanted to put the idea out. The branch announcement would be pretty straightforward.

@oraNod oraNod added the DaWGs Good discussion item for the DaWGs label Sep 22, 2024
@oraNod oraNod added the backport-2.18 Automatically create a backport for the stable-2.18 branch label Oct 24, 2024
@samccann samccann marked this pull request as draft November 18, 2024 16:47
@samccann
Copy link
Contributor Author

@oraNod finally getting back to this. I sort of understand what you are thinking in your last comment, but I don't know how to implement it, so if you want to give it a go, sure. I can close this PR.

The only requirement (other than make it easy pls :-) is that core and package docs EOL at very different times, and package EOL's first.

@oraNod oraNod mentioned this pull request Dec 2, 2024
Open
@oraNod
Copy link
Contributor

oraNod commented Dec 2, 2024

Thanks @samccann I can try and play around with this. Actually have an idea of how it might work without touching the makefile. I've put the comments and stuff in the GH issue for this if you want to go ahead and close.

@oraNod
Copy link
Contributor

oraNod commented Dec 2, 2024 •
edited
Loading

@samccann FYI, I've got something that seems like it will work. At least it inserts the banner during the build. I need to keep working on it and take it through some review and testing but I feel like it could be promising.

@samccann
Copy link
Contributor Author

samccann commented Dec 2, 2024

Cool! I'll close out this PR then.

@samccann samccann closed this Dec 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oraNod oraNod oraNod left review comments

@webknjaz webknjaz webknjaz left review comments

@gotmax23 gotmax23 Awaiting requested review from gotmax23

@felixfontein felixfontein Awaiting requested review from felixfontein

Assignees
No one assigned
Labels
backport-2.15 Automatically create a backport for the stable-2.15 branch backport-2.16 Automatically create a backport for the stable-2.16 branch backport-2.17 Automatically create a backport for the stable-2.17 branch backport-2.18 Automatically create a backport for the stable-2.18 branch DaWGs Good discussion item for the DaWGs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add a way to EOL package docs separate from core docs
3 participants
@samccann @oraNod @webknjaz